Name the argument when the constructor is given a non-string by dmccoystephenson · Pull Request #35 · Stephenson-Software/trace-client-python

dmccoystephenson · 2026-11-07T04:30:33Z

Summary

A truthy non-string base_url, application, version, key or install_id raised an AttributeError from .strip() that did not say which argument was wrong. A ValueError is now raised instead, such as version must be a string, not float, so the rule that constructor errors name their argument holds for these values too.
The message names the type and never the value, so a rejected key is not repeated in whatever logs the error.
Falsy values such as key=0 never reached .strip() and keep their meaning, and a test pins that.
One input that used to construct is now rejected: bytes (and any other truthy non-string with a .strip()). bytes for application, version, key or install_id built a disabled client without complaint, and key=b"k" built an enabled one, whose reports could not be sent because the Authorization header is built by adding it to a str. Such a program now stops at startup with a named error, including when reporting is off. That is accepted here because the same value would lose every report once reporting was on, and it matches the fix A non-string constructor argument raises an AttributeError that does not name the argument #33 suggests.
test_a_dropped_report_is_logged_at_debug_with_its_name is made deterministic. It failed in 2 of 7 full-suite runs on unmodified main, so it could have failed this PR at random. The wait is the one test_close_on_a_full_queue_drops_the_oldest_report_and_still_stops_the_sender already uses.
The README and the constructor docstring state the new error.

Closes #33
Closes #34

Test plan

python3 -m unittest on Python 3.8.10: 72 tests, OK, in 6 consecutive full-suite runs (69 tests before this change)
With the fix in trace_client/trace_client.py stashed, test_a_value_that_is_not_a_string_is_rejected_by_the_argument_it_was_given_for and test_a_rejected_key_is_not_repeated_in_the_error errored with AttributeError: 'int' object has no attribute 'strip'; with it restored, both passed
test_an_empty_value_of_another_type_keeps_its_old_meaning passed with the fix stashed and with it restored, as it characterizes behaviour that must not change
Forced race for test_a_dropped_report_is_logged_at_debug_with_its_name fails intermittently under load #34: with the sender held until just before overflow-event, the original test failed with 1 != 0 : a drop names the event it dropped: ['[trace] queue full, dropped filler'], and the rewritten test passed. The harness was a scratch file and is not committed
test (3.8), test (3.10) and test (3.12) green on the PR head (70caea5, run 42338761845)

Trace server and Java

No Java-visible behavior changed: nothing on the wire changes, and neither the opt-out values nor the disabled_reason strings are touched. This gap comes from Python's dynamic typing, so a counterpart in trace-client-java is not expected, but that source could not be read from this session and the point is unverified.

Issues not picked

A 301, 302 or 303 answer sends the reporting key to the redirect target as an empty GET #31: changes redirect behaviour in _send, which is wire format held for owner review, and it deserves its own PR.
The version tag is added even when it pushes an event past MAX_TAGS #16: still blocked on the owner decision and the trace-client-java lockstep recorded on that issue.

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

🤖 Generated with Claude Code

drafted by Claude on behalf of Daniel Stephenson

A truthy non-string base_url, application, version, key or install_id raised AttributeError from .strip() with no hint of which argument was wrong. They now raise a ValueError naming the argument and the type it got, never the value, so a rejected key does not reach a log. Falsy values keep their meaning: they never reached .strip(), so a program passing one already ran. Also wait for the sender to take the first report in test_a_dropped_report_is_logged_at_debug_with_its_name. If it woke after the last filler was dropped, the overflow event found a free slot and was queued, failing the test under load. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

dmccoystephenson · 2026-11-07T04:32:33Z

Self-review rubric (head 70caea5). This review was performed in the same session that wrote the change, so it is not an independent review.

Scope: PASS — three files changed. README.md, trace_client/trace_client.py and the three new tests serve A non-string constructor argument raises an AttributeError that does not name the argument #33; the rewritten test serves test_a_dropped_report_is_logged_at_debug_with_its_name fails intermittently under load #34, which was filed this cycle after the flake appeared on unmodified main.
Tests-new: PASS — no public name added; the new check is exercised through the constructor by three new tests.
Tests-fix: PASS — for A non-string constructor argument raises an AttributeError that does not name the argument #33, with trace_client/trace_client.py stashed, two new tests errored with AttributeError: 'int' object has no attribute 'strip', and both passed once restored. For test_a_dropped_report_is_logged_at_debug_with_its_name fails intermittently under load #34, a forced interleaving failed the original test with the message seen in the wild, and the rewritten test passed.
Sibling structure: PASS — no new files; the new tests sit beside test_constructor_errors_name_the_argument_they_reject.
Sibling renames: PASS — nothing renamed.
Docs: PASS — the README's version section and the constructor docstring state the error. The wire format, opt-out and promises sections are unaffected.
Issue resolution: PASS — all five arguments named in A non-string constructor argument raises an AttributeError that does not name the argument #33 are checked and the named test in test_a_dropped_report_is_logged_at_debug_with_its_name fails intermittently under load #34 is changed.
CI: PASS — test (3.8), test (3.10) and test (3.12) passed on run 42338761845.
Stdlib-only: PASS — no import added; pyproject.toml untouched.
Py3.8 floor: PASS — test (3.8) green on the PR head.
Never-raises / never-blocks: PASS — the check runs only in the constructor, the one path allowed to raise; report(), _drain(), _send() and close() are unchanged.
Log level: PASS — no _LOG call added.
Version agreement: PASS — 0.4.2 everywhere; no version string changed.
Exports: PASS — no public name added.
Java parity noted: PASS, with a caveat — the PR body states no Java-visible behavior changed, and that trace-client-java could not be read from this session.

Finding, corrected before posting: the first PR description said only values that already stopped the constructor change, and that was wrong. bytes, and any other truthy non-string with a .strip(), used to construct and is now rejected, including on a disabled client. The description was corrected to say so. The change is kept, since such a value loses every report once reporting is on.

Not changed: a falsy non-string such as key=0 still means no key, so an all-digit key of 0 in a settings file is silently a disabled client. That is existing behaviour, left alone so that no working program starts to stop at startup.

trace_client/trace_client.py:147 — the check precedes every .strip() and the opt-outs, so it also fires under DO_NOT_TRACK=1, like the existing blank and overlong checks.
tests/test_trace_client.py:478 — this new line is 116 columns, in a file that already has many lines past 110; left as is.

No do-not-auto-merge path matches: pyproject.toml, .github/workflows/, LICENSE, the version strings, _send, _json, the opt-out values and the disabled_reason strings are all untouched, and no file loses more than 50 lines.

This comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

drafted by Claude on behalf of Daniel Stephenson

dmccoystephenson merged commit 22199ca into main Nov 7, 2026
3 checks passed

dmccoystephenson deleted the trace/constructor-non-string-arguments branch November 7, 2026 04:32

Navigation Menu

Repository navigation

Name the argument when the constructor is given a non-string - #35

Name the argument when the constructor is given a non-string#35
dmccoystephenson merged 1 commit into
mainfrom
trace/constructor-non-string-arguments

dmccoystephenson commented Nov 7, 2026 •

edited

Loading

Uh oh!

dmccoystephenson commented Nov 7, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

dmccoystephenson commented Nov 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Trace server and Java

Issues not picked

Uh oh!

dmccoystephenson commented Nov 7, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

dmccoystephenson commented Nov 7, 2026 •

edited

Loading